Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add authorized_keys to TrustedUserCAKeys for certificate-based authen… #18

Closed

Conversation

backjo
Copy link

@backjo backjo commented Feb 5, 2021

…tication

Issue number:
#17

Description of changes:
Allows the authorized_keys file to also be used for certificate-based authentication. For larger organizations, certificate based authentication is desirable because it allows for short-lived credentials and allows for the CA to be the decider of who has access to which machines.

Testing done:

  • Tested ssh access on EC2 with both a public/private pair (existing use case) as well as with a different public/private pair signed by a CA (with the CA's public key being the one in authorized_keys)

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

sshd_config Outdated Show resolved Hide resolved
@jahkeup
Copy link
Member

jahkeup commented Feb 5, 2021

Always cool to see SSH-CA's being put to use @backjo 😺

That said, I'm not sure that this belongs in the base container image that we have today and that is used by default on Bottlerocket.

The TrustedUserCAKeys directive may induce logged warning and authentication "error" messages that don't apply to the typical use case - where its good 'ol .ssh/authorized_keys being used. These extra messages may confuse user and/or mislead monitoring infrastructure. With this being a more advanced, atypical configuration (taking advantage of IMDS carrying CA keys in - again, very cool!) I think that a custom image (for you) would be cleaner and clearer for your use-case.

Without a mechanism to optionally enable this feature, I'm hesitant to merge and enable this by default and for all instances of the admin host container.

@backjo
Copy link
Author

backjo commented Feb 5, 2021

Yeah - we're already using a custom image to achieve this, so it doesn't need to be merged and enabled to solve for us and our use case (though it would be nice to be on upstream :) ).

For the user experience, I don't think adding this would add too much confusion. The error messages (in my experience testing, at least) don't change just because of the presence of this setting if users are attempting to login with standard public/private keypairs. The only time that the user would get a new message was if they attempted to sign in with a certificate that was invalid (either expired, an invalid signer, or invalid principal, primarily) - in which case, I think it'd be acceptable to assume that the user is aware of SSH certificates and that the error would more useful than confusing.

I'd advocate that with the heavy focus on Security features in BottleRocket today (which is why we're leaning so heavily into it), having this feature would outweigh user confusion - at this point, mostly because it's unclear that any additional confusion would be caused for basic SSH users.

It also sounds like creating a mechanism to pass flags into the admin container (maybe passing TOML settings under a certain prefix as env variables?) might be something to open an issue for.

@tjkirch
Copy link
Contributor

tjkirch commented Feb 19, 2021

Hey @backjo! I wanted to follow up here because your use case is important to us, and I didn't want you to think we were just letting it sit.

We're currently working on some improvements to the admin container, and the interface between the container and Bottlerocket, to make things like this more sustainable. Specifically, we're moving some AWS-specific logic into Bottlerocket, so it doesn't have to be repeated by alternate host containers and to make supporting other platforms easier as well.

The admin container PR in particular handles adding TrustedUserCAKeys to sshd_config if the user specified a value in user data. (Similar to the flags you mentioned.) It's still undergoing some changes, but I'm hoping it will address your request here, and I'd be interested to get your thoughts on the approach.

@backjo
Copy link
Author

backjo commented Feb 19, 2021

Hey @tjkirch - appreciate the update! The PR looks good - UserData makes a lot of sense! Only thing I would be ever so slightly worried about is the impact of ssh keys there to the overall size of the user data. But directionally, it totally seems to be something we'd use and get value out of :)

@backjo backjo closed this Feb 19, 2021
@tjkirch
Copy link
Contributor

tjkirch commented Feb 19, 2021

@backjo Thanks! Yeah, user data is a limited resource, so we don't intend to store much there, but configuration that's important early in the startup process can be a good fit. (I'm working on bottlerocket-os/bottlerocket#1320 to allow compression and extend it a bit.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants